refactor: retire runners/lua_runner now that workflow_engine replaces it - #82
Merged
Conversation
Executes issue #73's decision record: prototypes/workflow_engine (via #74/#75, now fully landed) replaces runners/lua_runner as a whole rather than running alongside it as a second, parallel Lua execution path. Nothing in production depended on the SimpleCode+LUA path (it was only a few days old, never adopted for a real manifest), so this is a straight deletion, not a migration. Removed: - runners/lua_runner (crate deleted entirely), and its workspace member entry. - Its dependency and `lua` Cargo feature in binary/apid. - Its construction/registration in apid::construct_execution_engine and the now-dead LUA_LANG constant. - The `Ok(Language::LUA) => ...` arm of execution_engine::Engine::run's SimpleCode dispatch (a comment marks why it's gone, referencing #73). The LUA protobuf enum variant itself stays defined, per the issue's own plan - harmless, and a smaller footprint than changing a wire enum. - binary/apid/tests/lua_e2e.rs and the CI step that ran it (.github/workflows/rust.yml) - the only e2e coverage that existed specifically for the retired path. runners/workflow_runner's own tests used Language::LUA purely as a convenient "any SimpleCode language" stand-in for its api.run bridge tests (registering a FakeCodeRunner, never the real lua_runner) - switched to Language::JAVASCRIPT so those tests keep exercising Engine::run's real dispatch after the LUA arm is gone. Verified this still genuinely tests dispatch by temporarily registering under a mismatched key and confirming the test fails. Verified: cargo build/test --workspace clean, cargo build -p apid with every feature combination (including workflow) clean, cargo clippy --workspace --all-targets clean, cargo fmt --all -- --check clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019EKR96FrBygNE1sfscQ6CG
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #73. Executes its own decision record:
prototypes/workflow_engine(via #74/#75, now fully landed onmain) replacesrunners/lua_runneras a whole rather than running alongside it as a second, parallel Lua execution path. Nothing in production depended on theSimpleCode+LUApath (it was only a few days old, never adopted for a real manifest), so this is a straight deletion, not a migration — matching the issue's own reasoning.What changed
runners/lua_runner— crate deleted entirely, and its workspace member entry removed.binary/apid— its dependency andluaCargo feature removed; its construction/registration inconstruct_execution_engineremoved; the now-deadLUA_LANGconstant removed.execution_engine::Engine::run— theOk(Language::LUA) => ...arm of theSimpleCodedispatch match removed (a comment marks why, referencing Decision: retire runners/lua_runner once workflow_engine's async dispatch path lands #73). TheLUAprotobuf enum variant itself stays defined, per the issue's own plan — harmless, and a smaller footprint than changing a wire enum.binary/apid/tests/lua_e2e.rsand the CI step that ran it (.github/workflows/rust.yml) removed — the only e2e coverage that existed specifically for the retired path.runners/workflow_runner's own tests usedLanguage::LUApurely as a convenient "anySimpleCodelanguage" stand-in for itsapi.runbridge tests (registering aFakeCodeRunner, never the reallua_runner) — switched toLanguage::JAVASCRIPTso those tests keep exercisingEngine::run's real dispatch after theLUAarm is gone. Verified this still genuinely tests dispatch by temporarily registering under a mismatched key and confirming the test fails.Test plan
cargo build --workspaceandcargo test --workspaceclean.cargo build -p apidwith every feature combination (includingworkflow) clean.cargo clippy --workspace --all-targetsclean.cargo fmt --all -- --checkclean.lua_runner/Language::LUA/theluafeature remains anywhere in the repo outside historical precedent comments (which document design history and stay accurate) and the intentionally-retainedLUAproto enum variant.🤖 Generated with Claude Code
https://claude.ai/code/session_019EKR96FrBygNE1sfscQ6CG
Generated by Claude Code